[refactor] Single source of truth for trace span types - #5465
Conversation
The playwright suites are owned and type-checked by the tests workspace, which holds the @playwright/test dependency; tests/manual scripts import state modules that no longer exist.
- Restore Parameter, CorrectAnswer and _EvaluationScenario to lib/Types.ts (removed by cleanup while consumers remained) - Re-export MetricColumnDefinition from the EvalRunDetails table barrel - TooltipButtonProps was renamed EnhancedButtonProps; statusMap moved to @agenta/entity-ui/variant - Synthesize full Parameter objects in UseApiContent
- useRunMetricData: type selection as the unwrapped value, not the atom - Webhook builders: narrow WebhookFormValues union before field access - filtersAtom: accept updater functions in the write signature - Organization settings: drop stale react-query v4 useErrorBoundary and migrate setQueriesData to the v5 filters shape - evaluations/utils: surface variantId from invocation metadata - PreviewTableRow: add index signature required by InfiniteTableRowBase
Mirrors the vetted WP-4e-2a in-place fixes from fe-chore/move-evals-to-packages (adapted to OSS import paths) plus new fixes for the component layer: - restore PreviewTestCase to lib/Types; re-export MetricProcessor/MetricScope - add "input" to EvaluationColumnKind (backend mapping emits it) and type the visibility-label column extension in buildPreviewColumns - recharts v3 API drift in EvaluatorMetricsChart (TooltipContentProps, tuple radius, MetricStripEntry contextual typing) - widen evaluationType to EvaluationRunKind; type query atoms as nullable - latent runtime bugs typed as-is per WP-4e-2a convention (applyAggregatesToRaw, metricProcessor ReferenceErrors, dead metric-group branches) - flagged, not fixed
…ant id - createPaginatedEntityStore: refreshAtom/actions.refresh accept an optional value or updater (bare set() still bumps the counter) - clears the 'Expected 0 arguments' cluster across testset modals and other consumers - EnvironmentStatus: variant.id optional; the component already guards it
…ing layers; oss tsc 347->105 Parallel per-area pass, behavior-preserving throughout (latent runtime bugs are typed as-is with NOTE comments per the WP-4e-2a convention, not silently fixed): - TraceSpanNode OSS<->entities dual-type: aligned at every crossing with documented boundary casts (16 errors -> 0); annotations field added to the OSS node (drawer stores attach it at runtime) - SharedDrawers: broken SessionDrawerButton imports repaired, JSON-schema access typed, null-safety in SelectEvaluators/AnnotateDrawer, drawer payload/ref/label types aligned with runtime - observability: extended-column types for custom antd props, TraceRow/ SessionRow InfiniteTableRowBase conformance, traceTabsAtom updater support, ag-attributes selector typing at the producer - playground/url-state: removed drifted local duplicates of package types, eagerAtom->atom where deps are sync, modal/store prop alignment - onboarding/testsets/org: updater-widened widget UI atom, tour placement vocabulary fix, OnboardingLoader next/dynamic compat, org provider API types matched to the backend wire shape (settings/flags dicts) - EvalRunDetails remainder: WP-4e-2a-vetted atom fixes adapted, recharts v3 formatter/content signatures, stale import paths repointed Flagged for triage (typed as-is): AddToTestsetDrawer trace draft discard/ update call missing molecule actions; orphaned SessionDrawerButton
…atterns; oss tsc 105->82 - InfiniteVirtualTable: canonical ExtendedColumnType exported from the barrel (three local extended-column copies repointed as aliases); pure read fn for columnHiddenKeys (write path reconciles versions); memo generic preservation; onRow/getRowProps/rowSelection.fixed aligned with antd 6 - antd v6 PopoverStylesType 'body' drops typed as-is (2 sites, existing pattern) - React 19: JSX.Element -> ReactElement in RequireWorkflowKind - AgentaNodeDTO: trace_id/span_id added (backend SpanDTO sends them) - Org.default_workspace: list endpoint strips it - both lookups are latent dead code, typed as-is with comments - FiltersPreview: operatorLabel is a display string, not the operator union
…windowing export - DrillInUIContext: EditorProvider/SharedEditor slots typed with the real component prop types instead of a hand-written subset (root cause of the OSS provider assignment errors) - InfiniteVirtualTable: rowSelection.fixed accepts antd FixedType; locale accepted (not yet forwarded - flagged); Editor barrel exports CodeLanguage - workflow barrel exports WorkflowRevisionWindowing
Wave-2 parallel pass over the final ~105 OSS + 11 EE-only errors, behavior- preserving throughout (latent bugs typed as-is with NOTE comments per the WP-4e-2a convention): - Evaluators/Evaluations: generic evaluator filtering, chart datum typing - pages/evaluations: NewEvaluation modal props retyped to entities-package shapes (stale legacy Types imports dropped); antd 6 tabPlacement vocabulary - Testcases/Testsets/Deployments: canonical ExtendedColumnType adoption, dataset-store variance via Pick<...,'hooks'> - DrillInView/EditorViews: TMode narrowing via consts, Format|CodeLanguage state union, html format menu typing - app-shell misc: services/state/pages sweep with backend-verified API types - EE Billing + misc EE-only files Latent bugs surfaced and typed as-is (chips filed where actionable): workspace rename sends invalid org PATCH body; SessionInspector reads status.code the backend never sends; axios.isCancel dead on created instance; invite accept can interpolate undefined ids
Both apps are at zero tsc errors; flip ignoreBuildErrors to false so next build guards the baseline. Verified: full oss and ee builds pass with the gate on.
The OSS tracing module declared its own TraceSpan/TraceSpanNode plus four TypeScript enums that mirrored the zod schemas in @agenta/entities. The two were structurally equivalent but nominally incompatible, so every crossing needed a cast (7 of them, added in #5464 as documented scar tissue). - oss/src/services/tracing/types now owns only TraceSpanNode (entities node plus the annotations the drawer stores attach) and GenerationDashboardData; consumers import span types from @agenta/entities/trace directly, per the app-layer no-re-export rule - 25 enum value usages become string literals; StatusCode values were already identical, SpanCategory differed only in the catch-all - all 7 boundary casts removed Fixes a live crash: AGE-3788 canonicalised the backend catch-all from "undefined" to "unknown", but spanTypeStyles was still keyed "undefined" and both consumers destructure the lookup, so any span typed "unknown" threw a TypeError in the observability table and trace tree. The record is now keyed by SpanCategory (exhaustive, so a new backend category fails the build) with a fallback at both call sites. Nullability now matches the payload: entities declares | null where the backend sends null, so NodeNameCell/StatusRenderer/TimestampCell props and ScannedExportRow were widened rather than cast.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesTracing type alignment
Trace export deduplication
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/oss/src/services/tracing/types/index.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the source-of-truth comment.
This three-line explanation exceeds the repository rule for in-code comments; keep it to one short line or move the rationale to documentation.
As per coding guidelines, in-code comments must be at most one short line unless they document a genuinely surprising constraint.
Suggested wording
-// Span types have a single source of truth: the zod schemas in `@agenta/entities`, -// which validate the backend payload at the boundary. Import them from -// `@agenta/entities/trace` directly; this module owns only the OSS-only additions. +// Span types come from `@agenta/entities`; this module adds OSS annotations.Source: Coding guidelines
web/oss/src/components/pages/observability/assets/constants.ts (1)
1-3: 📐 Maintainability & Code Quality | 🔵 TrivialVerify the required frontend lint-fix command.
Before committing this frontend cohort, run
pnpm lint-fixfromweb; the supplied context does not confirm that exact command. As per coding guidelines, frontend changes must runpnpm lint-fixfromweb.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d6f21bdb-d182-4f40-b36c-46c0492e89b2
📒 Files selected for processing (19)
web/oss/src/components/EvalRunDetails/atoms/traces.tsweb/oss/src/components/SharedDrawers/SessionDrawer/store/sessionDrawerStore.tsweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/OverviewTabItem/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceContent/components/TraceTypeHeader/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceHeader/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTree/assets/spanVisibility.tsweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceTree/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/store/openInPlayground.tsweb/oss/src/components/SharedDrawers/TraceDrawer/store/traceDrawerStore.tsweb/oss/src/components/pages/observability/assets/constants.tsweb/oss/src/components/pages/observability/assets/exportUtils.tsweb/oss/src/components/pages/observability/components/AvatarTreeContent.tsxweb/oss/src/components/pages/observability/components/NodeNameCell.tsxweb/oss/src/components/pages/observability/components/StatusRenderer.tsxweb/oss/src/components/pages/observability/components/TimestampCell.tsxweb/oss/src/services/tracing/types/index.tsweb/oss/src/state/newObservability/atoms/queries.tsweb/oss/src/state/newObservability/atoms/queryHelpers.tsweb/packages/agenta-entities/src/trace/etl/exportMatchingTraces.ts
|
Web build is failing |
Conflicts were all in the same direction: release/v0.106.1 carries the pre-refactor span typing (#5464's boundary casts and narrow prop types) that this branch removes. Resolved to this branch's side in all six files. - services/tracing/types: keep the entities-backed TraceSpanNode; the release-side annotations field is already on it - OverviewTabItem: drop the EntityTraceSpan cast and its now-unused import - traceDrawerStore: SpanLink/TracesResponse come from @agenta/entities/trace - NodeNameCell/TimestampCell/formattedTimestampAtomFamily: keep the widened nullable types oss and ee tsc both at 0 errors after the merge.
Railway Preview Environment
Updated at 2026-07-29T10:52:26.802Z |
Review follow-up on the export ETL row contract. ScannedExportRow hand-redeclared five span fields, and this branch widened all five to `| null` to make TraceSpanNode assignable. Only three needed it: the schema declares trace_id and span_id required and non-null, so widening those two moved the contract away from the source of truth in the one file this PR was meant to unify. Pick the four span fields from TraceSpan instead. Same assignability for the caller, but the contract can no longer drift. That widening was what made the dedup collapse CodeRabbit flagged expressible: rows missing both ids all hashed to the fallback `trace_id:parent_id:start_time`, so a page of them deduped down to one row. Not reachable from the tracing API (zod validates both ids at the boundary), but the fallback was wrong on its own terms — it also collapsed sibling spans sharing a parent and start_time. Dedup may only drop what it can prove is a repeat, so a row with no identity is now passed through undeduped: for an export, a duplicate row beats a dropped one. Also shortens the span-types comment to the one-line repo rule. Test added for the no-identity path; it fails on the old key (1 row, not 3).
|
Rebased onto Merge conflicts ( @ashrafchowdury "web build is failing" — that was before the merge; the branch was stale against the release. All checks pass on the current head, including CodeRabbit's Nitpicks — shortened the span-types comment to the one-line rule in Verification on One judgment call worth a second opinion: making |
Context
A trace span was described by two type definitions: hand-written interfaces and TypeScript enums in
oss/src/services/tracing/types, and the zod schemas in@agenta/entities/tracethat actually validate the backend payload. They were structurally equivalent but nominally incompatible, so every place data crossed between them needed a cast. PR #5464 added seven such casts with// same backend span shapecomments. They were safe, but they meant a real backend shape change could pass through untyped.They had also already drifted. AGE-3788 canonicalised the backend catch-all category from
"undefined"to"unknown", and the entities schema followed. The OSS enum did not.Changes
@agenta/entities/traceis now the single source. The OSS module keeps only what the package cannot own:TraceSpanNode(the entities node plus theannotationsthe drawer stores attach at runtime, whoseAnnotationDtolives in the app layer) andGenerationDashboardData. Consumers import span types from the package directly rather than through a forwarding module, which is what the app-layer no-re-export lint rule asks for.The 25 enum value usages became string literals.
StatusCode's values were already byte-identical between the two definitions, andSpanCategorydiffered only in that catch-all. All seven boundary casts are gone.The drift was a live crash
spanTypeStyleswas keyed"undefined"and both consumers destructure the lookup:A span with
span_type: "unknown"misses the record, and destructuringundefinedthrows. That path reaches the observability table (NodeNameCell) and the trace tree (AvatarTreeContent). The record is now keyed bySpanCategory, so it is exhaustive and a new backend category fails the build, and both call sites fall back instead of throwing.Nullability
The entities schemas declare
| nullwhere the backend sends null. Rather than cast that away, the consumers that were under-declaring it were widened:NodeNameCell,StatusRenderer,TimestampCelland its atom, andScannedExportRowin the ETL pipeline (which only reads those fields).Tests / notes
pnpm --filter @agenta/oss exec tsc --noEmitandpnpm --filter @agenta/ee exec tsc --noEmit: 0 errors each.pnpm turbo run build --filter=@agenta/entitiesandpnpm turbo run lint --filter=@agenta/oss: clean.ts-chore/fix-tsc-issues.What to QA
ag.typeset (so the backend reports theunknowncategory), the observability table renders it with the generic icon instead of crashing. That is the bug this fixes.